Skip to content

fix(gateway): preserve Fable 1M picker selection - #3722

Merged
lidge-jun merged 2 commits into
devfrom
codex/lane-b-05-fable
Sep 5, 2026
Merged

fix(gateway): preserve Fable 1M picker selection#3722
lidge-jun merged 2 commits into
devfrom
codex/lane-b-05-fable

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Summary

Preserves separate Fable base and 1M picker selections and restores the reversible selector to canonical Fable before Messages and count_tokens native passthrough.

Carries #3649 by rebasing the original behavior commit 9a7795a and marked-request regression 284fe8c onto dev 330bf60. Both commits retain Éverton Toffanetto as author. The four-file feature delta is unchanged; no additional feature work is included. Supersedes #3649, to be closed after this replacement is verified on dev.

Verification

  • Rebase completed without conflicts. Inspected the four-file delta and original author metadata.
  • Source PR reports 28 focused tests and 1,543 changed tests passing, plus typecheck/privacy and a picker harness. Those are the source author's historical results, not new runs of this rebased head.
  • No local tests, suites, typecheck or builds were run. The maintainer explicitly requested dev rebase followed by admin merge, without waiting for fresh CI. No current-head runtime or CI success is claimed.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Co-authored-by: Éverton Toffanetto evertondgn@hotmail.com

Summary by CodeRabbit

  • New Features

    • Added separate Claude model picker entries for standard and 1M-context variants.
    • 1M variants now display a clear “1M” label and support up to 1,000,000 input tokens.
  • Bug Fixes

    • Improved handling of Claude model aliases so native routing remains consistent.
    • Fixed message and token-count requests using 1M aliases to forward correctly to the intended model.
  • Tests

    • Added coverage for model listings, native passthrough, and token-count requests.

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 5, 2026 23:41
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-05T23:45:04.874965Z 79d1a21 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@lidge-jun
lidge-jun merged commit 73190c2 into dev Sep 5, 2026
7 of 8 checks passed
@lidge-jun
lidge-jun deleted the codex/lane-b-05-fable branch September 5, 2026 23:41
@github-actions github-actions Bot added the bug Something isn't working label Sep 5, 2026
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: e9635856-5aa5-4632-8b51-9c2d51aff606

📥 Commits

Reviewing files that changed from the base of the PR and between 330bf60 and 79d1a21.

📒 Files selected for processing (4)
  • src/claude/model-info.ts
  • src/server/claude-messages.ts
  • tests/claude-integration/claude-model-info.test.ts
  • tests/claude-integration/claude-native-passthrough.test.ts

📝 Walkthrough

Walkthrough

The change separates readable Claude Code 1M model rows and decodes valid Fable picker aliases before Claude Messages routing, passthrough handling, and token estimation. Integration tests cover model listing and both Messages endpoints.

Changes

Claude Fable alias handling

Layer / File(s) Summary
Readable 1M model variants
src/claude/model-info.ts, tests/claude-integration/claude-model-info.test.ts
push1mVariant accepts an optional selector ID. Readable Anthropic rows use a canonical native alias before the [1m] marker. Tests verify separate base and 1M rows with max_input_tokens set to 1,000,000.
Fable alias decoding
src/server/claude-messages.ts
decodeFablePickerAlias restores valid claude-fable-* aliases before synthetic-row parsing, passthrough checks, routing, and token estimation for Messages and count_tokens.
Native passthrough validation
tests/claude-integration/claude-native-passthrough.test.ts
Tests verify passthrough for the picker alias with and without [1m], including response handling, token counting, upstream request count, and normalization to claude-fable-5-1.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ClaudeMessages
  participant AliasDecoder
  participant UpstreamClaude
  Client->>ClaudeMessages: Send picker alias
  ClaudeMessages->>AliasDecoder: Decode Fable alias
  AliasDecoder-->>ClaudeMessages: Return claude-fable-5-1
  ClaudeMessages->>UpstreamClaude: Forward normalized model
  UpstreamClaude-->>ClaudeMessages: Return response or token count
  ClaudeMessages-->>Client: Return result
Loading
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/lane-b-05-fable

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 70 / 80

이 PR은 Claude Code 피커에서 Fable 기본 줄과 1M 줄을 서로 다른 선택으로 남기는 고침입니다. 지금 dev(HEAD 2f124a167, 패키지 2.44.0, Aside GUI #3714 / Aside CLI #3710 / native function·namespace #3703 / exec patch #3701 스택 위)에서는 readable 스타일로 claude-fable-*를 올릴 때 1M 변형 id가 베이스id[1m] 형태입니다. Claude Code는 정규 Fable id를 [1m] 마커보다 먼저 묶어서, 기본과 1M이 한 선택처럼 보이거나 섞일 수 있습니다. 이 브랜치는 그 구멍을 막습니다.

고치는 곳은 두 갈래입니다. 첫째, src/claude/model-info.tspush1mVariant에 선택 id를 넘길 수 있게 합니다. readable + anthropic + claude-fable-일 때만 1M 줄 id를 claudeCodeNativeAlias(listedModelId) + [1m]으로 씁니다. 예: 베이스는 claude-fable-5-1, 1M은 claude-ocx-native--claude-fable-5-1[1m]. display_name은 그대로 … · 1M이고, 진짜 ≥1M 창이 있을 때만 올리는 #854 가드는 유지합니다. 둘째, src/server/claude-messages.tsdecodeFablePickerAlias를 넣고 Messages와 count_tokens 모두에서, 피커 별칭이 오면 Anthropic으로 넘기기 전에 정규 claude-fable-*로 되돌립니다. claudeCodeNativeAliasresolveInboundModel을 쓰므로 OpenAI-native 경로를 새로 만들지 않습니다.

테스트도 같이 옵니다. tests/claude-integration/claude-model-info.test.ts는 베이스/1M id가 갈라지는지 보고, claude-native-passthrough.test.ts는 마커 없는 별칭·[1m] 별칭·count_tokens 세 경로 모두 upstream model이 claude-fable-5-1로 나가는지 잡습니다. 본문은 #3649를 현재 dev 위로 리베이스한 교체본이고, Éverton Toffanetto 원 커밋을 유지한다고 적혀 있습니다. #3649는 이미 CLOSED입니다. 네 파일·기능 델타만이며 types.ts/config.ts 대분할에 통째로 무효화될 성격은 아닙니다. 본문이 말한 대로 이 헤드에서 새 CI/런타임 성공을 주장하지는 않습니다.

우선순위 70인 이유다. Fable 1M 피커는 사용자에게 바로 보이는 선택 버그이고, HEAD에 이미 있는 Claude Fable 시드(#3211 계열)와 잘 맞습니다. 다만 지금 dev가 최적화하는 축은 Aside 프로필·Grok catalog/patch 쪽이 더 앞에 있고, 이 PR은 그 열차와 겹치지 않는 작은 gateway 고침입니다. 범위가 좁고 회귀 테스트가 있어 머지 후보는 분명합니다.

경로/심볼 - src/claude/model-info.ts push1mVariant / oneMillionSelector - readable Fable만 native alias+[1m] selector; #854 ≥1M·중복 suffix 가드 유지
경로/심볼 - src/claude/alias.ts claudeCodeNativeAlias - 피커 별칭 생성에 재사용 (새 alias 규칙 추가 없음)
경로/심볼 - src/server/claude-messages.ts decodeFablePickerAlias - Messages·count_tokens에서 별칭을 정규 claude-fable-*로 복원 후 passthrough
경로 - tests/claude-integration/claude-model-info.test.ts - 베이스/1M id·display_name·max_input_tokens 분리 검증
경로 - tests/claude-integration/claude-native-passthrough.test.ts - 세 ingress 모두 upstream claude-fable-5-1
경로 - #3649 - 이미 CLOSED; 이 PR이 교체본. 랜딩 후 landed 주석만 확인하면 됨

메인테이너의 판단이 필요한 지점

  • 본문이 요청한 대로 fresh CI 없이 admin merge 할지, 아니면 claude-integration 조각만이라도 한 번 돌리고 올릴지
  • Fast selector(decodeClaudeFastSelector)와 Fable decode 순서가 운영에서 겹치는 입력을 만드는지 (코드상 Fable decode가 Fast 앞에 옴)
  • Desktop 3P(hashed) idStyle에는 이 selector가 적용되지 않음 — CLI readable만 고치는 범위가 의도대로 충분한지
  • #3649 CLOSED 상태에 Landed via #3722 주석·landed-via-maintainer를 보강할지

너의 추천
범위가 좁고 피커 회귀가 명확합니다. dev에 머지하세요. #3649는 이미 닫혀 있으니 랜딩 커밋 해시로 landed 주석만 보강하면 됩니다. types/config 대분할 close-don't-rebase 대상 아님. 미리보기 배포 없음.

이 댓글은 grok-bot이 작성했습니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants